Skip to content

Set attribute "data-node-id" on each node#21

Open
Geoffrey-D wants to merge 3 commits intoN00ts:masterfrom
Geoffrey-D:master
Open

Set attribute "data-node-id" on each node#21
Geoffrey-D wants to merge 3 commits intoN00ts:masterfrom
Geoffrey-D:master

Conversation

@Geoffrey-D
Copy link
Copy Markdown

This facilitate DOM manipulation :)

@N00ts
Copy link
Copy Markdown
Owner

N00ts commented Jul 23, 2022

Hello, since vue js is based on data mutation for DOM manipulation (virutal DOM), i'm not sure having an ID for manipulation is relevant. Why did you put "data-node-id" instead of a real :id ? What is your use case ?

@Geoffrey-D
Copy link
Copy Markdown
Author

Geoffrey-D commented Jul 23, 2022

I need to open and focus nodes programatically, and as no "API" allowed such things, I had to:

  • Modifying the tree props to set all parents as "opened"

  • Manipulating the DOM to manage the "focus" state (by toggling classes).

I have added in this MR 2 API methods to do those things in an "API" object:

this.$refs.Tree.API.open(id);
this.$refs.Tree.API.focus(id);

I have also changed the "data-node-id" attr to "id" as it makes more sense

@Geoffrey-D
Copy link
Copy Markdown
Author

What do you think of those 2 features? Can we merge them?

@Geoffrey-D
Copy link
Copy Markdown
Author

Hey @N00ts could you please give a feedback on this?

Thx :)

@N00ts
Copy link
Copy Markdown
Owner

N00ts commented Aug 16, 2022

Hey I had a look to your PR. I have multiple remarks.

  • Maybe just returning state in useTree would allow you to do whatever API you like (can $ref.tree.state in parent component).
  • If you really need those API, maybe just put them directly at root func of Tree (instead of tree.API.method). Can you also add unit tests (there is a yarn test) ?
  • I saw that dev.vue was modified, can you revert it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants